-
Notifications
You must be signed in to change notification settings - Fork 127
Improve the reliability of state changes in widget mode #3177
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
7924b3a to
e81dd84
Compare
e81dd84 to
c473d1f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like a simple patch PR should look like.
I am wondering if we can improve on the "custom js-sdk hash" situation?
package.json
Outdated
| "loglevel": "^1.9.1", | ||
| "matrix-js-sdk": "github:matrix-org/matrix-js-sdk#f3f4a1f7e2f5f2c26cf4fce89a729a9197f619b5", | ||
| "matrix-widget-api": "1.11.0", | ||
| "matrix-js-sdk": "github:matrix-org/matrix-js-sdk#7fa9195e398764749b0195f14d3cf0190ad10253", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this particular instance it might make sense to mention where exactly the js-sdk commit comes from.
I would almost argue we should make this a procedure rule:
- If one uses a commit NOT from the develop (main) branch, a link to the related branch with a short summary has to be added as a comment.
| "matrix-js-sdk": "github:matrix-org/matrix-js-sdk#7fa9195e398764749b0195f14d3cf0190ad10253", | |
| "NONMAIN-matrix-js-sdk":"This commit is from: https://github.com/matrix-org/matrix-js-sdk/pull/4847, the PR combines state-after and widget driver to-device fallback featurres.", | |
| "matrix-js-sdk": "github:matrix-org/matrix-js-sdk#7fa9195e398764749b0195f14d3cf0190ad10253", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I forgot its not that simple. NONMAIN-... will raise an error as the npm package cannot be found.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So we might need to find a good place somewhere else. package.json.md?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Different, possibly simpler idea:
Yarn has the ability to reference a Git dependency by branch name rather than commit. We can just use that! Then it's not too difficult to observe that we're using a non-develop branch and go over to https://github.com/matrix-org/matrix-js-sdk to dig into the details of the branch if you really need them.
I think Yarn has always had this feature, but in Yarn Classic it caused the package cache to be a bit broken and that's why we always avoided it. But now we're of course on Yarn Berry.
Rather than by commit, which makes it hard to tell whether we're using mainline matrix-js-sdk or not.
This makes the mechanism by which an Element Call widget can receive changes to room state more reliable by updating matrix-js-sdk and matrix-widget-api, which now support the
update_stateaction from the latest version of MSC2762.Depends on matrix-org/matrix-js-sdk#4790
For https://github.com/element-hq/voip-internal/issues/289